Skip to content

Dev#136

Merged
vvshk merged 27 commits into
mainfrom
dev
Jun 15, 2026
Merged

Dev#136
vvshk merged 27 commits into
mainfrom
dev

Conversation

@vvshk

@vvshk vvshk commented May 25, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a coordinator dashboard, coordinator login, and travel bus management system, including bus creation, passenger assignment, and bulk upload features. The review feedback highlights several critical issues: a runtime TypeError in travelBusDetails.js due to a missing DOM element, a UI-database sync issue in fetchUpcomingBookings.js when reassignment is cancelled, a potential date comparison failure in fetchUpcomingBookings.js, and a Cross-Site Scripting (XSS) vulnerability in coordinatorDashboard.js caused by rendering unescaped user input via innerHTML.

Comment thread admin/travel/travelBusDetails.js
Comment thread admin/travel/fetchUpcomingBookings.js
Comment thread admin/travel/fetchUpcomingBookings.js
Comment thread admin/travel/coordinatorDashboard.js
pallavisavla and others added 4 commits May 26, 2026 12:41
- Add multi-select dropdowns for starting_meal and ending_meal in create and update utsav forms
- Hide ending_meal when start and end dates are the same
- Hide both meal fields when location is not Research Centre
- Pre-fill meal selections on edit form

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-for-utsav

Feat/capture starting ending meal for utsav
@vvshk

vvshk commented May 29, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces comprehensive travel bus management features, including a coordinator dashboard, bus creation/editing, passenger assignment, and bulk upload capabilities, alongside updates to Utsav meal options and WiFi code request exports. Key feedback highlights a critical page-load crash in travelBusDetails.js due to a missing DOM element reference, high-severity XSS vulnerabilities in coordinatorDashboard.js and permanentCodeRequests.js from unsafe innerHTML usage, a state inconsistency bug in fetchUpcomingBookings.js during capacity upgrade prompts, and some dead code in travelBusDetails.js that should be removed.

Comment thread admin/travel/travelBusDetails.js
Comment thread admin/travel/coordinatorDashboard.js
Comment thread admin/wifi/permanentCodeRequests.js
Comment thread admin/travel/fetchUpcomingBookings.js
Comment thread admin/travel/travelBusDetails.js
@vvshk

vvshk commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

@cyrus-sratrc[bot] does this look good?

@cyrus-sratrc

cyrus-sratrc Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review: PR #136 (dev → main)

Large PR (27 files, +7.8k). Mostly a new bus-coordinator system (login/OTP, coordinator dashboard, bus management/details, bulk Excel upload) plus utsav meal fields, wifi router export, and a debounce on bulk-food updates.

Verdict: functional and consistent with existing patterns, but one recurring concern worth addressing before merge.

✅ Good

  • Auth wired correctly: coordinator pages use coordinatorToken + checkCoordinatorAuth(); admin pages use token. All fetches send the right Bearer.
  • No hardcoded API URLs — uses CONFIG.baseUrl / CONFIG.basePath correctly.
  • Error handling is consistent (!response.ok → throw → alert).
  • sudo cardno column + td:nth-child(4) selector bump is correct; utsav meal multi-selects fine; bulk-food debounce (optimistic UI + 2500ms + refetch on failure) is sound.

⚠️ Main concern — XSS (unescaped innerHTML)
API/Excel data is interpolated raw into markup across all new travel files. Highest risk:

  • Free-text comments field in coordinator dashboard.
  • Excel-sourced bus_name / stop_name in bulk-upload previews (attacker-controllable).
  • Attribute injection in inline onclick="...('${passenger_id}')" / exportBusPassengers('${bus_name}') — a quote in the value breaks the handler / injects JS.

This matches the existing codebase pattern, so it's not a regression — but the new bulk-Excel + free-text paths widen the surface. Recommend an escapeHtml() helper for text and addEventListener (or encoded args) for action links.

Minor

  • travelBusDetails.js: handleBulkUpload() is dead code (defined, never wired; different endpoint than the active inline flow).
  • coordinatorDashboard.js toggleBoardingStatus: doesn't handle 401/403, so expired token shows a parse error instead of logging out.
  • travelBusManagement.js: "No Data" colspan="4" wrong for the 3-col invalid table; confirmBulkMasterImport assumes a prior upload exists.
  • coordinatorLogin.js / coordinatorAuth.js missing trailing newline; utsav JS files gained exec bit (100755) — harmless.

Want me to apply fixes? I'd suggest the dead-code removal + 401/403 handling + an escapeHtml() pass on the new bulk/comments paths — happy to scope it tighter if you prefer minimal changes.

@vvshk vvshk merged commit cd57d30 into main Jun 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants